Skip to content

Night Shift: rename LanguageFilter "persian" to "original"#20

Open
EtanHey wants to merge 1 commit intomasterfrom
nightshift/2026-02-24-4248
Open

Night Shift: rename LanguageFilter "persian" to "original"#20
EtanHey wants to merge 1 commit intomasterfrom
nightshift/2026-02-24-4248

Conversation

@EtanHey
Copy link
Owner

@EtanHey EtanHey commented Feb 24, 2026

User description

Automated improvement by Golems Night Shift.

rename LanguageFilter "persian" to "original"


PR Type

Enhancement, Bug fix


Description

  • Rename LanguageFilter "persian" to "original" for multi-language support

  • Add backward compatibility layer normalizing stored "persian" values to "original"

  • Apply code formatting and style improvements across multiple files

  • Update test cases to reflect new "original" filter naming convention


Diagram Walkthrough

flowchart LR
  A["LanguageFilter Type"] -->|"Rename persian to original"| B["Updated Type Definition"]
  C["Stored Preferences<br/>persian value"] -->|"normalizeLanguageFilter<br/>function"| D["Normalized to original<br/>on read"]
  E["LyricsDisplay Component"] -->|"Use original filter"| F["Display original text"]
  G["Song Page"] -->|"Apply normalization"| H["Consistent filter handling"]
Loading

File Walkthrough

Relevant files
Enhancement
LyricsDisplay.tsx
Update LanguageFilter type and rename persian to original

src/components/LyricsDisplay.tsx

  • Renamed LanguageFilter type from "persian" to "original"
  • Updated filter condition checks from languageFilter === "persian" to
    languageFilter === "original"
  • Applied code formatting improvements for better readability
  • Updated JSX indentation and line breaks for consistency
+97/-96 
song.$songId.tsx
Add backward compatibility for language filter normalization

src/routes/song.$songId.tsx

  • Added normalizeLanguageFilter() function to convert legacy "persian"
    values to "original"
  • Applied function to user preferences initialization and sync effects
  • Updated language filter dropdown to display "original" option instead
    of "persian"
  • Applied extensive code formatting and import reorganization
  • Improved line wrapping and indentation throughout the file
+594/-364
Bug fix
useAnonymousProgress.ts
Normalize persian to original in progress validation         

src/hooks/useAnonymousProgress.ts

  • Added normalization logic in validateProgress() to convert stored
    "persian" values to "original"
  • Applied ternary operator to check and normalize languageFilter during
    data validation
  • Fixed code formatting with trailing commas and proper spacing
  • Improved function parameter formatting for consistency
+18/-16 
Tests
LyricsDisplay.test.tsx
Update test to use original filter naming                               

src/components/LyricsDisplay.test.tsx

  • Updated test case to use "original" filter instead of "persian"
  • Changed test description from "shows only Persian when filter is
    persian" to "shows only original text when filter is original"
  • Applied comprehensive code formatting with double quotes and
    semicolons
  • Improved test readability with better line wrapping and indentation
+233/-168
Formatting
songPageDefaults.test.ts
Apply code formatting improvements to test file                   

src/tests/songPageDefaults.test.ts

  • Applied code formatting improvements with consistent quote style and
    semicolons
  • Improved regex pattern formatting for better readability
  • Enhanced line wrapping for long test assertions
  • Standardized indentation and spacing throughout test file
+63/-46 

Summary by CodeRabbit

Release Notes

  • New Features

    • Enhanced playback controls with mode selection (single/loop/fluid) and improved preference persistence for playback speed and language filters.
    • Improved mobile and desktop presentation with dynamic layout adjustments.
  • Improvements

    • Renamed language view option from "Persian" to "Original" for clarity.
    • Better handling of progress tracking across authenticated and anonymous users.
  • Tests

    • Expanded test coverage for language rendering, line interactions, and RTL text handling.

The LanguageFilter type hardcoded "persian" as the value for showing
original/source text, but the app supports multiple source languages
(Korean, Arabic, Hebrew, etc.). Renamed to "original" for clarity.

Includes backward compatibility: stored "persian" preferences (Convex DB
and localStorage) are normalized to "original" on read.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vercel
Copy link

vercel bot commented Feb 24, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
songscript Ready Ready Preview, Comment Feb 24, 2026 2:09am

@coderabbitai
Copy link

coderabbitai bot commented Feb 24, 2026

📝 Walkthrough

Walkthrough

The PR renames the "persian" language filter option to "original" across the codebase, introduces backward-compatibility normalization for the renamed filter, expands test coverage for the LyricsDisplay component, applies quote style formatting improvements, and significantly enhances the song page with new playback controls, mode selection UI, and improved progress orchestration logic.

Changes

Cohort / File(s) Summary
Test Formatting & Coverage
src/__tests__/songPageDefaults.test.ts, src/components/LyricsDisplay.test.tsx
Quote style normalization (single to double quotes) and minor syntax adjustments; LyricsDisplay tests expanded to cover language filtering, RTL handling, active/clicked line highlighting, line click callbacks, and visibility toggling across Persian, transliteration, Hebrew, and English variants.
LyricsDisplay Component
src/components/LyricsDisplay.tsx
Public type LanguageFilter updated from "all" | "persian" | "transliteration" | "hebrew" | "english" to "all" | "original" | "transliteration" | "hebrew" | "english"; rendering logic gated by "original" instead of "persian"; quote style and trailing comma adjustments.
Progress & State Management
src/hooks/useAnonymousProgress.ts
Language filter normalization added: maps legacy "persian" string to "original"; minor formatting adjustments to function signatures and dependency arrays; error handling unchanged.
Song Page Route
src/routes/song.$songId.tsx
Major refactoring introducing backward-compat normalizeLanguageFilter function, new UI toggles for playback modes (single/loop/fluid) and language/video controls, updated progress orchestration distinguishing authenticated vs anonymous flows with optimistic line-learned updates, expanded import statements, and refactored event handlers and time-seeking logic.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Poem

🐰 The language now speaks "original," not "persian" by name,
While playback modes dance—fluid, loop, and single's flame,
Tests bloom with coverage, from Athens to the shore,
Quote marks unified, and progress tracks what's more! ✨

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: renaming the LanguageFilter value from 'persian' to 'original' throughout the codebase, which is the primary objective reflected in commit messages and file modifications.
Docstring Coverage ✅ Passed Docstring coverage is 85.71% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch nightshift/2026-02-24-4248

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@qodo-code-review
Copy link

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

🔴
Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Unhandled async failures: Newly added Convex mutations and promise chains (e.g.,
toggleWordLearnedMutation(...).then(...) and preference updates) lack catch/error
handling, risking silent failures and inconsistent optimistic UI state.

Referred Code
const persistPlaybackSpeed = useCallback(
  (speed: string) => {
    if (!isAuthenticated) return;
    updatePreferencesMutation({ playbackSpeed: parseFloat(speed) });
  },
  [updatePreferencesMutation, isAuthenticated],
);

const persistLanguageFilter = useCallback(
  (filter: LanguageFilter) => {
    if (!isAuthenticated) return;
    updatePreferencesMutation({ languageFilter: filter });
  },
  [updatePreferencesMutation, isAuthenticated],
);

const persistPlaybackMode = useCallback(
  (mode: PlaybackMode) => {
    if (!isAuthenticated) return;
    updatePreferencesMutation({ playbackMode: mode });
  },


 ... (clipped 389 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status:
Missing input validation: The PR parses and persists playbackSpeed via parseFloat(speed) without validating allowed
values or guarding against NaN, potentially storing/propagating invalid data to
preferences.

Referred Code
const persistPlaybackSpeed = useCallback(
  (speed: string) => {
    if (!isAuthenticated) return;
    updatePreferencesMutation({ playbackSpeed: parseFloat(speed) });
  },
  [updatePreferencesMutation, isAuthenticated],
);

const persistLanguageFilter = useCallback(
  (filter: LanguageFilter) => {
    if (!isAuthenticated) return;
    updatePreferencesMutation({ languageFilter: filter });
  },
  [updatePreferencesMutation, isAuthenticated],
);

const persistPlaybackMode = useCallback(
  (mode: PlaybackMode) => {
    if (!isAuthenticated) return;
    updatePreferencesMutation({ playbackMode: mode });
  },


 ... (clipped 28 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Missing audit context: The PR introduces new preference/progress mutations but the diff does not show whether
these actions are captured in immutable audit logs with user identity, timestamp, action,
and outcome.

Referred Code
const persistPlaybackSpeed = useCallback(
  (speed: string) => {
    if (!isAuthenticated) return;
    updatePreferencesMutation({ playbackSpeed: parseFloat(speed) });
  },
  [updatePreferencesMutation, isAuthenticated],
);

const persistLanguageFilter = useCallback(
  (filter: LanguageFilter) => {
    if (!isAuthenticated) return;
    updatePreferencesMutation({ languageFilter: filter });
  },
  [updatePreferencesMutation, isAuthenticated],
);

const persistPlaybackMode = useCallback(
  (mode: PlaybackMode) => {
    if (!isAuthenticated) return;
    updatePreferencesMutation({ playbackMode: mode });
  },


 ... (clipped 389 lines)

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status:
Error detail exposure: The new console.warn("Local video error:", error) may expose internal error
strings to end users via browser console depending on what error contains.

Referred Code
const handleVideoError = useCallback((error: string) => {
  setVideoError(error);
  console.warn("Local video error:", error);
}, []);

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status:
Unstructured console logs: New console.warn logging is unstructured and may include sensitive or internal details in
the logged error payload depending on its source.

Referred Code
const handleVideoError = useCallback((error: string) => {
  setVideoError(error);
  console.warn("Local video error:", error);
}, []);

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Save preferences for anonymous users

Update persistLanguageFilter to save preferences for anonymous users to
localStorage by calling progress.updatePreferences.

src/routes/song.$songId.tsx [444-450]

 const persistLanguageFilter = useCallback(
   (filter: LanguageFilter) => {
-    if (!isAuthenticated) return;
-    updatePreferencesMutation({ languageFilter: filter });
+    if (isAuthenticated) {
+      updatePreferencesMutation({ languageFilter: filter });
+    } else {
+      progress.updatePreferences({ languageFilter: filter });
+    }
   },
-  [updatePreferencesMutation, isAuthenticated],
+  [updatePreferencesMutation, isAuthenticated, progress],
 );
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: This is a valid bug fix that correctly identifies that language filter preferences for anonymous users are not being persisted, and it provides the correct implementation to fix it.

Medium
Persist playback speed for all users

Update persistPlaybackSpeed to save preferences for anonymous users to
localStorage by calling progress.updatePreferences.

src/routes/song.$songId.tsx [436-442]

 const persistPlaybackSpeed = useCallback(
   (speed: string) => {
-    if (!isAuthenticated) return;
-    updatePreferencesMutation({ playbackSpeed: parseFloat(speed) });
+    if (isAuthenticated) {
+      updatePreferencesMutation({ playbackSpeed: parseFloat(speed) });
+    } else {
+      progress.updatePreferences({ playbackSpeed: parseFloat(speed) });
+    }
   },
-  [updatePreferencesMutation, isAuthenticated],
+  [updatePreferencesMutation, isAuthenticated, progress],
 );
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: This is a valid bug fix that correctly identifies that playback speed preferences for anonymous users are not being persisted, and it provides the correct implementation to fix it.

Medium
Save video mute state for everyone

Update persistVideoMuted to save preferences for anonymous users to localStorage
by calling progress.updatePreferences.

src/routes/song.$songId.tsx [460-466]

 const persistVideoMuted = useCallback(
   (muted: boolean) => {
-    if (!isAuthenticated) return;
-    updatePreferencesMutation({ videoMuted: muted });
+    if (isAuthenticated) {
+      updatePreferencesMutation({ videoMuted: muted });
+    } else {
+      progress.updatePreferences({ videoMuted: muted });
+    }
   },
-  [updatePreferencesMutation, isAuthenticated],
+  [updatePreferencesMutation, isAuthenticated, progress],
 );
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: This is a valid bug fix that correctly identifies that video mute preferences for anonymous users are not being persisted, and it provides the correct implementation to fix it.

Medium
Persist video collapsed state for all

Update persistVideoCollapsed to save preferences for anonymous users to
localStorage by calling progress.updatePreferences.

src/routes/song.$songId.tsx [468-474]

 const persistVideoCollapsed = useCallback(
   (collapsed: boolean) => {
-    if (!isAuthenticated) return;
-    updatePreferencesMutation({ videoCollapsed: collapsed });
+    if (isAuthenticated) {
+      updatePreferencesMutation({ videoCollapsed: collapsed });
+    } else {
+      progress.updatePreferences({ videoCollapsed: collapsed });
+    }
   },
-  [updatePreferencesMutation, isAuthenticated],
+  [updatePreferencesMutation, isAuthenticated, progress],
 );
  • Apply / Chat
Suggestion importance[1-10]: 8

__

Why: This is a valid bug fix that correctly identifies that video collapsed preferences for anonymous users are not being persisted, and it provides the correct implementation to fix it.

Medium
General
Validate stored filter value

Validate the languageFilter value from localStorage against the allowed
LanguageFilter types, defaulting to "all" if it's invalid.

src/hooks/useAnonymousProgress.ts [215-220]

 languageFilter:
-      typeof p.languageFilter === "string"
-        ? p.languageFilter === "persian"
-          ? "original"
-          : p.languageFilter
-        : "all",
+  (() => {
+    const raw = typeof p.languageFilter === "string" ? p.languageFilter : undefined;
+    const mapped = raw === "persian" ? "original" : raw;
+    const valid: LanguageFilter[] = ["all","original","transliteration","hebrew","english"];
+    return valid.includes(mapped as any) ? (mapped as LanguageFilter) : "all";
+  })(),
  • Apply / Chat
Suggestion importance[1-10]: 6

__

Why: The suggestion correctly points out a missing validation step and provides a robust implementation to ensure that only valid LanguageFilter values are stored, improving data integrity.

Low
  • More

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
src/components/LyricsDisplay.tsx (1)

83-83: 🧹 Nitpick | 🔵 Trivial

lineRefs is typed as HTMLButtonElement[] but stores <div> elements.

The ref array is typed as (HTMLButtonElement | null)[] (line 83), and the cast on line 105 forces el as HTMLButtonElement | null. However, the actual element is a <div> (line 102). Since only scrollIntoView is called (which exists on all HTMLElements), this isn't a runtime bug, but the typing is misleading.

Suggested fix
- const lineRefs = useRef<(HTMLButtonElement | null)[]>([]);
+ const lineRefs = useRef<(HTMLDivElement | null)[]>([]);
- lineRefs.current[index] = el as HTMLButtonElement | null;
+ lineRefs.current[index] = el;

Also applies to: 102-106

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/LyricsDisplay.tsx` at line 83, The ref array lineRefs is
incorrectly typed as (HTMLButtonElement | null)[] while it stores div elements;
update the type used in the useRef declaration (lineRefs) to (HTMLElement |
null)[] (or a more specific HTMLDivElement | null[]) and adjust any casts (the
setter that currently uses el as HTMLButtonElement | null) to use the matching
HTMLElement/HTMLDivElement type so calls like scrollIntoView remain correctly
typed; locate useRef(...) for lineRefs and the ref assignment where el is cast
and change both to the consistent HTMLElement/HTMLDivElement type.
src/routes/song.$songId.tsx (1)

382-409: 🧹 Nitpick | 🔵 Trivial

Practice logging fires a Convex mutation every second per active user.

logPracticeMutation({ eventType: "audio_time", value: 1 }) is called every second for authenticated users. At scale, this could create significant write load on Convex. Consider batching (e.g., accumulate locally and flush every 10-30 seconds) to reduce mutation frequency.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/routes/song`.$songId.tsx around lines 382 - 409, The effect currently
calls logPracticeMutation({ eventType: "audio_time", value: 1 }) every second
for authenticated users which will cause high write volume; change useEffect
logic (the effect using lastActivityRef, IDLE_THRESHOLD_MS, shouldCountTime,
practiceSecondsRef, isAuthenticated, logPracticeMutation, logPracticeFn) to
accumulate seconds in a local counter (e.g., practiceBatchRef) and only call
logPracticeMutation with the batched value on a longer flush interval (e.g.,
10–30s) or when user becomes idle/unmounts, while keeping logPracticeFn behavior
for anonymous users; ensure you flush remaining batched seconds on
cleanup/unload and reset counters appropriately so real-time header updates
remain reasonably fresh but mutations are throttled.
src/hooks/useAnonymousProgress.ts (1)

12-19: 🧹 Nitpick | 🔵 Trivial

Consider renaming WordProgressItem.persian field to original for consistency.

The persian field in WordProgressItem stores the word's text in the source language. Given this PR's goal of supporting multiple source languages (Korean, Arabic, Hebrew), the field name persian is misleading for non-Persian songs. This would require a data migration for existing localStorage entries, similar to what's been done for languageFilter.

This is not blocking since it's a data schema concern beyond this PR's scope, but worth tracking.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/hooks/useAnonymousProgress.ts` around lines 12 - 19, The WordProgressItem
interface in useAnonymousProgress.ts uses a Persian-specific field name
(WordProgressItem.persian); rename this field to a neutral name like original
across the type and all usages (serialization/deserialization, reads/writes, and
any accessors in useAnonymousProgress) and add a simple migration when loading
saved progress from localStorage to map old persian keys to original (similar to
the languageFilter migration) so existing data is preserved; update all code
that reads/writes WordProgressItem (including any JSON.parse/JSON.stringify
paths) to handle both old and new property names until migration is complete.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/components/LyricsDisplay.test.tsx`:
- Around line 1-57: Add tests in LyricsDisplay.test.tsx to cover the checkbox
and info-button interactions: render LyricsDisplay (wrapped with
QueryClientProvider if needed), pass vi.fn() spies for onLineCheckboxClick and
onLineInfoClick props, use testing-library fireEvent or userEvent to click the
checkbox for a specific lyric and the info button for that lyric, and assert the
corresponding spy was called with the expected lyric id or payload; locate
handlers by prop names onLineCheckboxClick and onLineInfoClick and target
elements by role/label/text matching the line content used in the mocked data.

In `@src/components/LyricsDisplay.tsx`:
- Line 57: Replace the hardcoded "persian" fallback used when computing RTL with
the new renamed filter value so unknown source languages default to LTR: update
the isOriginalRTL assignment (the variable isOriginalRTL that calls
isRTLLanguage(sourceLanguage || "persian")) to use "original" (or otherwise pass
an empty/undefined value) instead of "persian" so the RTL check aligns with the
rename and treats unknown languages as LTR.

In `@src/routes/song`.$songId.tsx:
- Around line 822-844: The handleToggleWordLearned callback calls
toggleWordLearnedMutation but never handles failures; update
handleToggleWordLearned to add a .catch on the toggleWordLearnedMutation promise
(or use try/catch if converting to async) that logs the error (e.g.,
console.error or processLogger), shows user-facing feedback (toast or set an
error state) and, if you applied any optimistic UI changes, revert them;
reference the toggleWordLearnedMutation call inside handleToggleWordLearned and
also keep logPracticeMutation and toggleWordLearnedFn behavior unchanged for the
success path.
- Around line 506-511: Refactor duplicated seek-guard blocks to use the existing
seekTo helper: replace the inline pattern in handlePlaybackModeChange,
handleLyricLineClick, and handleWordModalClose with calls to
seekTo(sortedLyrics[lineIndexToPlay].startTime) (or the appropriate startTime
for the clicked/closed line) and remove the manual isSeekingRef/current and
setTimeout logic; to fix handlePlaybackModeChange you must move the seekTo
function definition above handlePlaybackModeChange (or hoist it) and then add
seekTo to handlePlaybackModeChange’s dependency array so hooks remain correct;
keep references to playerRef, isSeekingRef, sortedLyrics and lineIndexToPlay as
before.

---

Outside diff comments:
In `@src/components/LyricsDisplay.tsx`:
- Line 83: The ref array lineRefs is incorrectly typed as (HTMLButtonElement |
null)[] while it stores div elements; update the type used in the useRef
declaration (lineRefs) to (HTMLElement | null)[] (or a more specific
HTMLDivElement | null[]) and adjust any casts (the setter that currently uses el
as HTMLButtonElement | null) to use the matching HTMLElement/HTMLDivElement type
so calls like scrollIntoView remain correctly typed; locate useRef(...) for
lineRefs and the ref assignment where el is cast and change both to the
consistent HTMLElement/HTMLDivElement type.

In `@src/hooks/useAnonymousProgress.ts`:
- Around line 12-19: The WordProgressItem interface in useAnonymousProgress.ts
uses a Persian-specific field name (WordProgressItem.persian); rename this field
to a neutral name like original across the type and all usages
(serialization/deserialization, reads/writes, and any accessors in
useAnonymousProgress) and add a simple migration when loading saved progress
from localStorage to map old persian keys to original (similar to the
languageFilter migration) so existing data is preserved; update all code that
reads/writes WordProgressItem (including any JSON.parse/JSON.stringify paths) to
handle both old and new property names until migration is complete.

In `@src/routes/song`.$songId.tsx:
- Around line 382-409: The effect currently calls logPracticeMutation({
eventType: "audio_time", value: 1 }) every second for authenticated users which
will cause high write volume; change useEffect logic (the effect using
lastActivityRef, IDLE_THRESHOLD_MS, shouldCountTime, practiceSecondsRef,
isAuthenticated, logPracticeMutation, logPracticeFn) to accumulate seconds in a
local counter (e.g., practiceBatchRef) and only call logPracticeMutation with
the batched value on a longer flush interval (e.g., 10–30s) or when user becomes
idle/unmounts, while keeping logPracticeFn behavior for anonymous users; ensure
you flush remaining batched seconds on cleanup/unload and reset counters
appropriately so real-time header updates remain reasonably fresh but mutations
are throttled.

ℹ️ Review info

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9a964b0 and 2125f37.

📒 Files selected for processing (5)
  • src/__tests__/songPageDefaults.test.ts
  • src/components/LyricsDisplay.test.tsx
  • src/components/LyricsDisplay.tsx
  • src/hooks/useAnonymousProgress.ts
  • src/routes/song.$songId.tsx
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Cursor Bugbot
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use TanStack Start framework with Bun runtime for the application

Tests must pass locally via bun run test before committing code, as Husky pre-commit hooks will block commits with failing tests

Files:

  • src/routes/song.$songId.tsx
  • src/components/LyricsDisplay.test.tsx
  • src/__tests__/songPageDefaults.test.ts
  • src/components/LyricsDisplay.tsx
  • src/hooks/useAnonymousProgress.ts
**/*.tsx

📄 CodeRabbit inference engine (CLAUDE.md)

Components with logic MUST have corresponding test files following the naming convention ComponentName.test.tsx

Files:

  • src/routes/song.$songId.tsx
  • src/components/LyricsDisplay.test.tsx
  • src/components/LyricsDisplay.tsx
**/*.test.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use Vitest for unit testing and Playwright for integration testing

Files:

  • src/components/LyricsDisplay.test.tsx
  • src/__tests__/songPageDefaults.test.ts
**/*.test.tsx

📄 CodeRabbit inference engine (CLAUDE.md)

Component tests should be located in the same directory as the component file with naming convention ComponentName.test.tsx

Files:

  • src/components/LyricsDisplay.test.tsx
**/*.ts

📄 CodeRabbit inference engine (CLAUDE.md)

All new helpers and utilities MUST have corresponding test files

Files:

  • src/__tests__/songPageDefaults.test.ts
  • src/hooks/useAnonymousProgress.ts
**/*.test.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Unit tests for utilities should be located in the same directory as the utility file with naming convention utilName.test.ts

Files:

  • src/__tests__/songPageDefaults.test.ts
src/__tests__/**/*.test.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Integration tests should be located in src/__tests__/ directory with naming convention featureName.test.ts

Files:

  • src/__tests__/songPageDefaults.test.ts
🧠 Learnings (12)
📚 Learning: 2026-01-28T12:17:53.877Z
Learnt from: CR
Repo: EtanHey/songscript PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-28T12:17:53.877Z
Learning: Applies to app/routes/**/*.tsx : Use TanStack file-based routing convention with route files like index.tsx, login.tsx, and song.$songId.tsx

Applied to files:

  • src/routes/song.$songId.tsx
📚 Learning: 2026-01-23T18:12:38.519Z
Learnt from: CR
Repo: EtanHey/songscript PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-23T18:12:38.519Z
Learning: Applies to app/routes/**/*.tsx : Use TanStack file-based routing for routes defined in app/routes/ directory

Applied to files:

  • src/routes/song.$songId.tsx
📚 Learning: 2026-01-28T12:17:53.877Z
Learnt from: CR
Repo: EtanHey/songscript PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-28T12:17:53.877Z
Learning: Applies to convex/**/*.ts : Use Convex schema.ts for database schema definitions and songs.ts for queries and mutations

Applied to files:

  • src/routes/song.$songId.tsx
  • src/__tests__/songPageDefaults.test.ts
📚 Learning: 2026-01-23T18:12:38.519Z
Learnt from: CR
Repo: EtanHey/songscript PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-23T18:12:38.519Z
Learning: Applies to convex/*.ts : Use Convex for database queries, mutations, and authentication configuration

Applied to files:

  • src/routes/song.$songId.tsx
📚 Learning: 2026-01-28T12:17:53.877Z
Learnt from: CR
Repo: EtanHey/songscript PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-28T12:17:53.877Z
Learning: Applies to **/*.tsx : Components with logic MUST have corresponding test files following the naming convention `ComponentName.test.tsx`

Applied to files:

  • src/components/LyricsDisplay.test.tsx
  • src/__tests__/songPageDefaults.test.ts
📚 Learning: 2026-01-23T18:12:38.519Z
Learnt from: CR
Repo: EtanHey/songscript PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-23T18:12:38.519Z
Learning: Applies to **/*.test.{ts,tsx} : Use Vitest for unit testing and Playwright for integration testing

Applied to files:

  • src/components/LyricsDisplay.test.tsx
  • src/__tests__/songPageDefaults.test.ts
📚 Learning: 2026-01-28T12:17:53.877Z
Learnt from: CR
Repo: EtanHey/songscript PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-28T12:17:53.877Z
Learning: Applies to **/*.ts : All new helpers and utilities MUST have corresponding test files

Applied to files:

  • src/__tests__/songPageDefaults.test.ts
📚 Learning: 2026-01-28T12:17:53.877Z
Learnt from: CR
Repo: EtanHey/songscript PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-28T12:17:53.877Z
Learning: Applies to src/__tests__/**/*.test.ts : Integration tests should be located in `src/__tests__/` directory with naming convention `featureName.test.ts`

Applied to files:

  • src/__tests__/songPageDefaults.test.ts
📚 Learning: 2026-01-28T12:17:53.877Z
Learnt from: CR
Repo: EtanHey/songscript PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-28T12:17:53.877Z
Learning: Applies to **/*.test.ts : Unit tests for utilities should be located in the same directory as the utility file with naming convention `utilName.test.ts`

Applied to files:

  • src/__tests__/songPageDefaults.test.ts
📚 Learning: 2026-01-28T12:17:53.877Z
Learnt from: CR
Repo: EtanHey/songscript PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-28T12:17:53.877Z
Learning: Applies to **/*.test.tsx : Component tests should be located in the same directory as the component file with naming convention `ComponentName.test.tsx`

Applied to files:

  • src/__tests__/songPageDefaults.test.ts
📚 Learning: 2026-01-28T12:17:53.877Z
Learnt from: CR
Repo: EtanHey/songscript PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-01-28T12:17:53.877Z
Learning: Song transliteration learning app should display song lyrics line by line with transliteration and allow users to follow along and learn pronunciation

Applied to files:

  • src/components/LyricsDisplay.tsx
📚 Learning: 2026-01-23T18:12:49.193Z
Learnt from: CR
Repo: EtanHey/songscript PR: 0
File: prd-json/AGENTS.md:0-0
Timestamp: 2026-01-23T18:12:49.193Z
Learning: Applies to prd-json/**/convex/**/*{score,progress,leaderboard}*.{ts,tsx,js} : Use Progress Score Formula: (words_learned × multiplier) + (lines_completed × multiplier × 0.5)

Applied to files:

  • src/hooks/useAnonymousProgress.ts
🧬 Code graph analysis (2)
src/routes/song.$songId.tsx (5)
src/components/LyricsDisplay.tsx (2)
  • LanguageFilter (21-26)
  • LyricLine (9-19)
src/components/LanguageFlag.tsx (1)
  • isRTLLanguage (85-90)
src/components/ui/select.tsx (5)
  • Select (178-178)
  • SelectTrigger (186-186)
  • SelectValue (187-187)
  • SelectContent (179-179)
  • SelectItem (181-181)
src/components/dashboard/LanguageChip.tsx (1)
  • getLanguageDisplayName (17-36)
src/components/dashboard/index.tsx (1)
  • getLanguageDisplayName (56-56)
src/components/LyricsDisplay.test.tsx (1)
src/components/LyricsDisplay.tsx (1)
  • LyricsDisplay (46-190)
🔇 Additional comments (6)
src/components/LyricsDisplay.tsx (1)

21-26: LGTM — LanguageFilter type correctly renamed from "persian" to "original".

The type update is clean and aligns with the PR objective.

src/hooks/useAnonymousProgress.ts (1)

216-220: LGTM — Backward-compatible normalization of stored "persian" to "original".

The nested ternary correctly handles the migration path: "persian""original", other strings pass through, non-strings default to "all".

src/routes/song.$songId.tsx (2)

52-63: LGTM — normalizeLanguageFilter provides clean backward compatibility.

Correctly maps legacy "persian" to "original", validates known filter values, and falls back to "all".


1116-1119: Good UX: dynamic "Original" label uses getLanguageDisplayName.

The language filter dropdown shows the actual source language name (e.g., "Persian Only", "Korean Only") instead of a generic "Original Only", which is a nice touch for multi-language support.

src/components/LyricsDisplay.test.tsx (1)

169-192: Good coverage of the renamed "original" filter.

The test correctly verifies that languageFilter="original" shows only the original text and hides transliteration, Hebrew, and English. This directly validates the core rename from "persian" to "original".

src/__tests__/songPageDefaults.test.ts (1)

1-48: These tests are structure assertions without coverage for the normalizeLanguageFilter function; consider adding behavior-based testing.

The tests read and match exact substrings from the source file—a brittle pattern by design. The file correctly uses Vitest and follows integration test conventions. However, the normalizeLanguageFilter function (which handles the "persian" → "original" mapping) is not tested anywhere. Since this function is private and not exported, direct unit testing is not feasible. Instead, consider testing the backward-compatibility behavior indirectly through page rendering assertions or adding a dedicated integration test that verifies the language filter logic through user interactions (e.g., setting preferences with deprecated "persian" value and confirming it's normalized to "original").

Comment on lines +1 to +57
import { describe, it, expect, vi, beforeEach } from "vitest";
import { render, screen, fireEvent } from "@testing-library/react";
import { QueryClient, QueryClientProvider } from "@tanstack/react-query";
import LyricsDisplay from "./LyricsDisplay";

// Mock the convexQuery function
vi.mock('@convex-dev/react-query', () => ({
vi.mock("@convex-dev/react-query", () => ({
convexQuery: vi.fn(() => ({
queryKey: ['lyrics', 'test-song-id'],
queryKey: ["lyrics", "test-song-id"],
queryFn: vi.fn(),
})),
}))
}));

// Mock useSuspenseQuery to return mock lyrics data
vi.mock('@tanstack/react-query', async (importOriginal) => {
const actual = await importOriginal<typeof import('@tanstack/react-query')>()
vi.mock("@tanstack/react-query", async (importOriginal) => {
const actual = await importOriginal<typeof import("@tanstack/react-query")>();
return {
...actual,
useSuspenseQuery: vi.fn(() => ({
data: [
{
_id: 'lyric1',
songId: 'test-song-id',
_id: "lyric1",
songId: "test-song-id",
lineNumber: 1,
startTime: 14.81,
endTime: 17.46,
original: 'برای توی کوچه رقصیدن',
transliteration: 'Barāye tūye kūche raqsidan',
hebrew: 'בָּרָאיֶה טוּיֶה כּוּצֶ׳ה רַקְסִידַן',
english: 'For dancing in the alley',
original: "برای توی کوچه رقصیدن",
transliteration: "Barāye tūye kūche raqsidan",
hebrew: "בָּרָאיֶה טוּיֶה כּוּצֶ׳ה רַקְסִידַן",
english: "For dancing in the alley",
},
{
_id: 'lyric2',
songId: 'test-song-id',
_id: "lyric2",
songId: "test-song-id",
lineNumber: 2,
startTime: 17.46,
endTime: 20.91,
original: 'برای ترسیدن به وقت بوسیدن',
transliteration: 'Barāye tarsidan be vaqt-e būsidan',
hebrew: 'בָּרָאיֶה טַרְסִידַן בֶּה וַקְטֶה בּוּסִידַן',
english: 'For being afraid at the moment of kissing',
original: "برای ترسیدن به وقت بوسیدن",
transliteration: "Barāye tarsidan be vaqt-e būsidan",
hebrew: "בָּרָאיֶה טַרְסִידַן בֶּה וַקְטֶה בּוּסִידַן",
english: "For being afraid at the moment of kissing",
},
{
_id: 'lyric3',
songId: 'test-song-id',
_id: "lyric3",
songId: "test-song-id",
lineNumber: 3,
startTime: 20.91,
endTime: 24.63,
original: 'برای خواهرم خواهرت خواهرامون',
transliteration: 'Barāye khāharam khāharet khāharāmūn',
original: "برای خواهرم خواهرت خواهرامون",
transliteration: "Barāye khāharam khāharet khāharāmūn",
// No Hebrew for this line to test optional field
english: 'For my sister, your sister, our sisters',
english: "For my sister, your sister, our sisters",
},
],
})),
}
})
};
});
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick | 🔵 Trivial

Tests don't cover the new checkbox/learned-state UI or info button.

The LyricsDisplay component now has significant interactive elements (checkbox toggle, info button) that aren't covered by these tests. While those elements may predate this PR, the coding guidelines state that "Components with logic MUST have corresponding test files." Consider adding tests for onLineCheckboxClick and onLineInfoClick callbacks. As per coding guidelines, **/*.tsx: "Components with logic MUST have corresponding test files following the naming convention ComponentName.test.tsx".

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/LyricsDisplay.test.tsx` around lines 1 - 57, Add tests in
LyricsDisplay.test.tsx to cover the checkbox and info-button interactions:
render LyricsDisplay (wrapped with QueryClientProvider if needed), pass vi.fn()
spies for onLineCheckboxClick and onLineInfoClick props, use testing-library
fireEvent or userEvent to click the checkbox for a specific lyric and the info
button for that lyric, and assert the corresponding spy was called with the
expected lyric id or payload; locate handlers by prop names onLineCheckboxClick
and onLineInfoClick and target elements by role/label/text matching the line
content used in the mocked data.

@@ -51,27 +56,27 @@ export default function LyricsDisplay({
}: LyricsDisplayProps) {
const isOriginalRTL = isRTLLanguage(sourceLanguage || "persian");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Hardcoded "persian" fallback is inconsistent with the rename.

The PR renames the language filter from "persian" to "original" throughout, but this line still defaults to "persian" when sourceLanguage is undefined. While isRTLLanguage does handle "persian" correctly, this is the one remaining hardcoded "persian" string in the component. If the app is meant to support Korean, Arabic, Hebrew, etc. as source languages, defaulting to "persian" may also produce incorrect RTL detection for non-Persian songs that lack sourceLanguage.

Suggested fix
- const isOriginalRTL = isRTLLanguage(sourceLanguage || "persian");
+ const isOriginalRTL = isRTLLanguage(sourceLanguage || "");

This way, songs without a sourceLanguage default to LTR (which is the safer fallback for unknown languages), rather than assuming Persian/RTL.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/components/LyricsDisplay.tsx` at line 57, Replace the hardcoded "persian"
fallback used when computing RTL with the new renamed filter value so unknown
source languages default to LTR: update the isOriginalRTL assignment (the
variable isOriginalRTL that calls isRTLLanguage(sourceLanguage || "persian")) to
use "original" (or otherwise pass an empty/undefined value) instead of "persian"
so the RTL check aligns with the rename and treats unknown languages as LTR.

Comment on lines +506 to +511
isSeekingRef.current = true;
playerRef.current?.seekTo(sortedLyrics[lineIndexToPlay].startTime);
setTimeout(() => {
isSeekingRef.current = false;
}, 200);
playerRef.current?.play();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Get file size first to understand scope
wc -l src/routes/song.$songId.tsx

Repository: EtanHey/songscript

Length of output: 113


🏁 Script executed:

# Read the specific sections mentioned in the review
# Lines 506-511 (snippet in review)
sed -n '500,515p' src/routes/song.$songId.tsx | cat -n

echo "---"
# Lines 615-619 (mentioned pattern)
sed -n '610,625p' src/routes/song.$songId.tsx | cat -n

echo "---"
# Lines 643 (seekTo helper)
sed -n '640,650p' src/routes/song.$songId.tsx | cat -n

echo "---"
# Lines 773-777 (mentioned pattern)
sed -n '768,785p' src/routes/song.$songId.tsx | cat -n

echo "---"
# Lines 517-525 (dependency array)
sed -n '515,530p' src/routes/song.$songId.tsx | cat -n

Repository: EtanHey/songscript

Length of output: 414


🏁 Script executed:

# Search for all instances of the manual seek-with-guard pattern
rg -n 'isSeekingRef\.current = true' src/routes/song.\$songId.tsx
echo "---"
rg -n 'isSeekingRef\.current = false' src/routes/song.\$songId.tsx

Repository: EtanHey/songscript

Length of output: 397


🏁 Script executed:

fd 'song.*songId.tsx' src/routes/

Repository: EtanHey/songscript

Length of output: 89


🏁 Script executed:

# List files in src/routes/ to see the actual filename
ls -la src/routes/ | grep song

Repository: EtanHey/songscript

Length of output: 129


🏁 Script executed:

# Read the sections with proper escaping
sed -n '500,520p' 'src/routes/song.$songId.tsx' | cat -n
echo "---LINES 615-620---"
sed -n '610,625p' 'src/routes/song.$songId.tsx' | cat -n
echo "---LINES 640-655---"
sed -n '640,655p' 'src/routes/song.$songId.tsx' | cat -n
echo "---LINES 770-780---"
sed -n '770,780p' 'src/routes/song.$songId.tsx' | cat -n

Repository: EtanHey/songscript

Length of output: 2593


🏁 Script executed:

# Find the seekTo function definition
rg -n 'const seekTo' 'src/routes/song.$songId.tsx'

Repository: EtanHey/songscript

Length of output: 114


🏁 Script executed:

# Find the handlePlaybackModeChange function and its dependency array
rg -n 'handlePlaybackModeChange' 'src/routes/song.$songId.tsx' -A 30

Repository: EtanHey/songscript

Length of output: 4255


🏁 Script executed:

# Get the full dependency array for handlePlaybackModeChange
sed -n '487,530p' 'src/routes/song.$songId.tsx' | cat -n

Repository: EtanHey/songscript

Length of output: 1689


🏁 Script executed:

# Check context around the second instance (lines 610-625) to understand which callback it is
sed -n '600,630p' 'src/routes/song.$songId.tsx' | cat -n

Repository: EtanHey/songscript

Length of output: 1102


🏁 Script executed:

# Check context around the fourth instance (lines 765-785)
sed -n '760,790p' 'src/routes/song.$songId.tsx' | cat -n

Repository: EtanHey/songscript

Length of output: 1324


Duplicated seek-with-guard pattern — refactor to use the seekTo helper.

The seekTo helper (line 643) encapsulates the isSeekingRef guard + setTimeout clear pattern, but this same pattern is manually inlined at:

  • Lines 506–511 in handlePlaybackModeChange
  • Lines 615–620 in handleLyricLineClick
  • Lines 773–778 in handleWordModalClose

This creates a maintenance risk if the guard timing needs adjusting.

However, note that seekTo is defined after handlePlaybackModeChange (line 487). To refactor handlePlaybackModeChange, you'll need to either move the seekTo definition before it or restructure the code. The other two instances (in handleLyricLineClick and handleWordModalClose) can use seekTo directly since they're defined after it. If refactoring handlePlaybackModeChange, add seekTo to its dependency array once defined.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/routes/song`.$songId.tsx around lines 506 - 511, Refactor duplicated
seek-guard blocks to use the existing seekTo helper: replace the inline pattern
in handlePlaybackModeChange, handleLyricLineClick, and handleWordModalClose with
calls to seekTo(sortedLyrics[lineIndexToPlay].startTime) (or the appropriate
startTime for the clicked/closed line) and remove the manual
isSeekingRef/current and setTimeout logic; to fix handlePlaybackModeChange you
must move the seekTo function definition above handlePlaybackModeChange (or
hoist it) and then add seekTo to handlePlaybackModeChange’s dependency array so
hooks remain correct; keep references to playerRef, isSeekingRef, sortedLyrics
and lineIndexToPlay as before.

Comment on lines +822 to +844
const handleToggleWordLearned = useCallback(
(wordId: Id<"words">, persian: string) => {
if (isAuthenticated) {
// Authenticated: use Convex mutation
toggleWordLearnedMutation({ wordId, persian }).then(
(newLearnedState) => {
if (newLearnedState) {
logPracticeMutation({ eventType: "word_learned", value: 1 });
}
},
);
} else {
// Anonymous: use localStorage via useProgress hook
toggleWordLearnedFn(persian, wordId);
}
},
[
isAuthenticated,
toggleWordLearnedMutation,
logPracticeMutation,
toggleWordLearnedFn,
],
);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Missing error handling on toggleWordLearnedMutation promise.

The .then() handles the success case, but there's no .catch(). If the mutation fails, the user gets no feedback and the word's learned state may be out of sync.

Suggested fix
       toggleWordLearnedMutation({ wordId, persian }).then(
         (newLearnedState) => {
           if (newLearnedState) {
             logPracticeMutation({ eventType: "word_learned", value: 1 });
           }
         },
-      );
+      ).catch((err) => {
+        console.error("Failed to toggle word learned state:", err);
+      });
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const handleToggleWordLearned = useCallback(
(wordId: Id<"words">, persian: string) => {
if (isAuthenticated) {
// Authenticated: use Convex mutation
toggleWordLearnedMutation({ wordId, persian }).then(
(newLearnedState) => {
if (newLearnedState) {
logPracticeMutation({ eventType: "word_learned", value: 1 });
}
},
);
} else {
// Anonymous: use localStorage via useProgress hook
toggleWordLearnedFn(persian, wordId);
}
},
[
isAuthenticated,
toggleWordLearnedMutation,
logPracticeMutation,
toggleWordLearnedFn,
],
);
const handleToggleWordLearned = useCallback(
(wordId: Id<"words">, persian: string) => {
if (isAuthenticated) {
// Authenticated: use Convex mutation
toggleWordLearnedMutation({ wordId, persian }).then(
(newLearnedState) => {
if (newLearnedState) {
logPracticeMutation({ eventType: "word_learned", value: 1 });
}
},
).catch((err) => {
console.error("Failed to toggle word learned state:", err);
});
} else {
// Anonymous: use localStorage via useProgress hook
toggleWordLearnedFn(persian, wordId);
}
},
[
isAuthenticated,
toggleWordLearnedMutation,
logPracticeMutation,
toggleWordLearnedFn,
],
);
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/routes/song`.$songId.tsx around lines 822 - 844, The
handleToggleWordLearned callback calls toggleWordLearnedMutation but never
handles failures; update handleToggleWordLearned to add a .catch on the
toggleWordLearnedMutation promise (or use try/catch if converting to async) that
logs the error (e.g., console.error or processLogger), shows user-facing
feedback (toast or set an error state) and, if you applied any optimistic UI
changes, revert them; reference the toggleWordLearnedMutation call inside
handleToggleWordLearned and also keep logPracticeMutation and
toggleWordLearnedFn behavior unchanged for the success path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant